Repository navigation
feat: introduce all-one-one (auth vs noauth) multi-stage build - #821
Conversation
🦋 Changeset detectedLatest commit: f04669b The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| @@ -0,0 +1,11 @@ | |||
| #!/bin/bash | |||
| # Meant to be run from the root of the repo | |||
There was a problem hiding this comment.
You could harden this bash script to always execute from the repo root directory using git and pushd:
# move to the repo base and remember the current directory
pushd $(git rev-parse --show-toplevel) > /de/null
# On exit, call popd to restore the prior working directory.
trap "popd > /dev/null" EXIT
# do work knowing the cwd is the repo base directory
docker build --squash ......
Probably not critical but provides a nicer user experience.
There was a problem hiding this comment.
I'm also curious why this is a shell script but the other docker build commands are in the Makefile. Putting it into the Makefile keeps the commands in a single place.
There was a problem hiding this comment.
good point. this guy is just for local testing. I can move it to Makefile
There was a problem hiding this comment.
will cleanup the Makefile later
There was a problem hiding this comment.
historical context is that I'm not a huge fan of make - its ergonomics are pretty poor as a command line runner as it's meant to be a build tool, at the time we settled on commands can be bash files that can be called in from make or from npm scripts which is why I wrote this in a script for the initial version of local mode
| # Set up App (copy from prod stage) | ||
| COPY --from=prod /app/api /app/api | ||
| COPY --from=prod /app/app /app/app | ||
| COPY --from=hyperdx ./entry.local.sh /etc/local/entry.base.sh |
There was a problem hiding this comment.
From what I can see, this is the only place that copies a source file into /etc/local/entry.base.sh within the docker container. Why not just name the source file /entry.base.sh to make it easier to follow the flow.
Right now the auth/noauth entry point scripts call something named entry.base.sh and it becomes another step to search and figure out that this file was actually a renamed docker/hyperdx/entry.local.sh version.
There was a problem hiding this comment.
I was debating this name. The reason behind entry.local.sh is that it's used for 'local mode', which spins up additional services compared to entry.prod.sh. Maybe entry.local.base.sh would be a better name?
|
|
||
| # all-in-one no auth ############################################################################################ | ||
| FROM all-in-one-base AS all-in-one-noauth | ||
| COPY --from=hyperdx ./entry.local.noauth.sh /etc/local/entry.sh |
There was a problem hiding this comment.
The auth/noauth local files only set a single, environment variable. Could we just use the ENV command here to set the value and avoid additional shell script files?
There was a problem hiding this comment.
The whole point is to prevent users from overriding the environment variable to potentially disable auth
1. Merge 'fullstack' and 'local' (auth + noauth) builds into a single Dockerfile 2. Introduce 'all-in-one-auth' and 'all-in-one-noauth' build stages 3. Lock `IS_LOCAL_APP_MODE` env var 4. Fix bug in ctrl-c exit with docker run 5. Enable alerts in local mode (no-auth) 6. Build `common-utils` on the fly (no longer needing pulling pkg from npm) Ref: HDX-1709 Ref: HDX-1713 Ref: HDX-1254 Ref: HDX-1729 To match v2 product definition, we are going to release three images: - hyperdx/hyperdx (--target=prod): app only without any other deps (clickhouse, otelcol, mongodb), used in default compose + helm deployment - hyperdx/hyperdx-all-in-one (--target=all-in-one-auth): all-in-one build + auth - hyperdx/hyperdx-local (--target=all-in-one-noauth): all-in-one build + no-auth Production impacts: - hyperdx/hyperdx: none - hyperdx/hyperdx-all-in-one: new - hyperdx/hyperdx-local: add server components (alerts, saved searches, dashboards)
IS_LOCAL_APP_MODEenv varcommon-utilson the fly (no longer needing pulling pkg from npm)Ref: HDX-1709
Ref: HDX-1713
Ref: HDX-1254
Ref: HDX-1729
To match v2 product definition, we are going to release three images:
Production impacts: